Skip to content

Commit

Permalink
Move limit to constant
Browse files Browse the repository at this point in the history
  • Loading branch information
@iamacook
iamacook committed Oct 14, 2024
1 parent c3d1fe5 commit 62a6e13
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 90 deletions.
1 change: 0 additions & 1 deletion src/config/entities/__tests__/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ export default (): ReturnType<typeof configuration> => ({
safeConfig: {
baseUri: faker.internet.url({ appendSlash: false }),
chains: {
maxLimit: faker.number.int(),
maxSequentialPages: faker.number.int(),
},
},
Expand Down
3 changes: 0 additions & 3 deletions src/config/entities/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,6 @@ export default () => ({
baseUri:
process.env.SAFE_CONFIG_BASE_URI || 'https://safe-config.safe.global/',
chains: {
// According to the limits of the Config Service
// @see https://github.com/safe-global/safe-config-service/blob/main/src/chains/views.py#L14-L16
maxLimit: parseInt(process.env.SAFE_CONFIG_CHAINS_MAX_LIMIT ?? `${40}`),
maxSequentialPages: parseInt(
process.env.SAFE_CONFIG_CHAINS_MAX_SEQUENTIAL_PAGES ?? `${3}`,
),
Expand Down
186 changes: 108 additions & 78 deletions src/domain/chains/chains.repository.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,17 @@ const mockConfigurationService = jest.mocked({
* tests are required to cover all edge cases.
*/
describe('ChainsRepository', () => {
let target: ChainsRepository;
// According to the limits of the Config Service
// @see https://github.com/safe-global/safe-config-service/blob/main/src/chains/views.py#L14-L16
const OFFSET = 40;

const maxLimit = 40;
let target: ChainsRepository;
const maxSequentialPages = 3;

beforeEach(() => {
jest.resetAllMocks();

mockConfigurationService.getOrThrow.mockImplementation((key) => {
if (key === 'safeConfig.chains.maxLimit') return maxLimit;
if (key === 'safeConfig.chains.maxSequentialPages')
return maxSequentialPages;
});
Expand All @@ -55,37 +56,46 @@ describe('ChainsRepository', () => {
});

it('should return all chains across pages below request limit', async () => {
const offset = 40;
const url = faker.internet.url({ appendSlash: true });
const chains = Array.from(
{
length: maxLimit * (maxSequentialPages - 1), // One page less than request limit
length: ChainsRepository.MAX_LIMIT * (maxSequentialPages - 1), // One page less than request limit
},
(_, i) => chainBuilder().with('chainId', i.toString()).build(),
);
const pages = chunk(chains, maxLimit).map((results, i, arr) => {
const pageOffset = (i + 1) * offset;
const pages = chunk(chains, ChainsRepository.MAX_LIMIT).map(
(results, i, arr) => {
const pageOffset = (i + 1) * OFFSET;

const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();

return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
});
return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
},
);
mockConfigApi.getChains.mockImplementation(
({ offset }): Promise<Page<Chain>> => {
if (offset === 0) {
Expand Down Expand Up @@ -123,37 +133,46 @@ describe('ChainsRepository', () => {
});

it('should return all chains across pages up request limit', async () => {
const offset = 40;
const url = faker.internet.url({ appendSlash: true });
const chains = Array.from(
{
length: maxLimit * maxSequentialPages, // Exactly request limit
length: ChainsRepository.MAX_LIMIT * maxSequentialPages, // Exactly request limit
},
(_, i) => chainBuilder().with('chainId', i.toString()).build(),
);
const pages = chunk(chains, maxLimit).map((results, i, arr) => {
const pageOffset = (i + 1) * offset;
const pages = chunk(chains, ChainsRepository.MAX_LIMIT).map(
(results, i, arr) => {
const pageOffset = (i + 1) * OFFSET;

const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();

return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
});
return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
},
);
mockConfigApi.getChains.mockImplementation(
({ offset }): Promise<Page<Chain>> => {
if (offset === 0) {
Expand Down Expand Up @@ -195,37 +214,46 @@ describe('ChainsRepository', () => {
});

it('should return all chains across pages up to request limit and notify if there are more', async () => {
const offset = 40;
const url = faker.internet.url({ appendSlash: true });
const chains = Array.from(
{
length: maxLimit * (maxSequentialPages + 1), // One page more than request limit
length: ChainsRepository.MAX_LIMIT * (maxSequentialPages + 1), // One page more than request limit
},
(_, i) => chainBuilder().with('chainId', i.toString()).build(),
);
const pages = chunk(chains, maxLimit).map((results, i, arr) => {
const pageOffset = (i + 1) * offset;
const pages = chunk(chains, ChainsRepository.MAX_LIMIT).map(
(results, i, arr) => {
const pageOffset = (i + 1) * OFFSET;

const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(maxLimit, pageOffset, url);
})();
const previous = ((): string | null => {
if (i === 0) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();
const next = ((): string | null => {
if (i === arr.length - 1) {
return null;
}
return limitAndOffsetUrlFactory(
ChainsRepository.MAX_LIMIT,
pageOffset,
url,
);
})();

return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
});
return pageBuilder<Chain>()
.with('results', results)
.with('count', chains.length)
.with('previous', previous)
.with('next', next)
.build();
},
);
mockConfigApi.getChains.mockImplementation(
({ offset }): Promise<Page<Chain>> => {
if (offset === 0) {
Expand All @@ -244,12 +272,14 @@ describe('ChainsRepository', () => {
const result = await target.getAllChains();

expect(result).toStrictEqual(
chains.slice(0, maxLimit * maxSequentialPages).map((chain) => {
return {
...chain,
ensRegistryAddress: getAddress(chain.ensRegistryAddress!),
};
}),
chains
.slice(0, ChainsRepository.MAX_LIMIT * maxSequentialPages)
.map((chain) => {
return {
...chain,
ensRegistryAddress: getAddress(chain.ensRegistryAddress!),
};
}),
);
expect(mockConfigApi.getChains).toHaveBeenNthCalledWith(1, {
limit: 40,
Expand Down
10 changes: 5 additions & 5 deletions src/domain/chains/chains.repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import { IConfigurationService } from '@/config/configuration.service.interface'

@Injectable()
export class ChainsRepository implements IChainsRepository {
private readonly maxLimit: number;
// According to the limits of the Config Service
// @see https://github.com/safe-global/safe-config-service/blob/main/src/chains/views.py#L14-L16
static readonly MAX_LIMIT = 40;

private readonly maxSequentialPages: number;

constructor(
Expand All @@ -32,9 +35,6 @@ export class ChainsRepository implements IChainsRepository {
@Inject(IConfigurationService)
private readonly configurationService: IConfigurationService,
) {
this.maxLimit = this.configurationService.getOrThrow<number>(
'safeConfig.chains.maxLimit',
);
this.maxSequentialPages = this.configurationService.getOrThrow<number>(
'safeConfig.chains.maxSequentialPages',
);
Expand Down Expand Up @@ -68,7 +68,7 @@ export class ChainsRepository implements IChainsRepository {
let next = null;

for (let i = 0; i < this.maxSequentialPages; i++) {
const result = await this.getChains(this.maxLimit, offset);
const result = await this.getChains(ChainsRepository.MAX_LIMIT, offset);

next = result.next;
chains.push(...result.results);
Expand Down
7 changes: 4 additions & 3 deletions src/routes/owners/owners.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import {
import { TestQueuesApiModule } from '@/datasources/queues/__tests__/test.queues-api.module';
import { QueuesApiModule } from '@/datasources/queues/queues-api.module';
import type { Server } from 'net';
import { ChainsRepository } from '@/domain/chains/chains.repository';

describe('Owners Controller (Unit)', () => {
let app: INestApplication<Server>;
let safeConfigUrl: string;
let maxLimit: number;
let networkService: jest.MockedObjectDeep<INetworkService>;

beforeEach(async () => {
Expand All @@ -52,7 +52,6 @@ describe('Owners Controller (Unit)', () => {
IConfigurationService,
);
safeConfigUrl = configurationService.getOrThrow('safeConfig.baseUri');
maxLimit = configurationService.getOrThrow('safeConfig.chains.maxLimit');
networkService = moduleFixture.get(NetworkService);

app = await new TestAppProvider().provide(moduleFixture);
Expand Down Expand Up @@ -386,7 +385,9 @@ describe('Owners Controller (Unit)', () => {
expect(networkService.get).toHaveBeenCalledTimes(1);
expect(networkService.get).toHaveBeenCalledWith({
url: `${safeConfigUrl}/api/v1/chains`,
networkRequest: { params: { limit: maxLimit, offset: 0 } },
networkRequest: {
params: { limit: ChainsRepository.MAX_LIMIT, offset: 0 },
},
});
});

Expand Down

0 comments on commit 62a6e13

Please sign in to comment.