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

refactor: Better Result class implementation #23335

Merged
merged 38 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a07e18f
feat: Tranforms and fallbacks for promise-based `Result<T, E>`
zharinov Jul 12, 2023
0419325
Export `ResultPromise`
zharinov Jul 12, 2023
25d86ee
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 12, 2023
c84119a
Refactor
zharinov Jul 12, 2023
2ae5b38
Rename to `AsyncResult`
zharinov Jul 12, 2023
f17ed87
More refactoring
zharinov Jul 13, 2023
3789cf6
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 13, 2023
1220e87
Improve destructuring
zharinov Jul 13, 2023
708af10
Fix error handling
zharinov Jul 13, 2023
78425d6
Fix for rubygems
zharinov Jul 13, 2023
ee4848a
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 13, 2023
4f06ac7
Refactor
zharinov Jul 14, 2023
9a4f14f
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 14, 2023
c5b4025
Move Result handling out of `try-catch` block
zharinov Jul 14, 2023
63a7667
Fix wrap types
zharinov Jul 14, 2023
45c8622
Enforce `NonNullable` constraint
zharinov Jul 14, 2023
7c6896b
Add `wrapNullable`
zharinov Jul 14, 2023
98e09ff
Enforce non-nullable error types
zharinov Jul 14, 2023
e392297
Fix lint
zharinov Jul 14, 2023
5703a8a
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 14, 2023
2955956
Refactor
zharinov Jul 14, 2023
76c1154
Refactor
zharinov Jul 14, 2023
195ece5
Add docs
zharinov Jul 15, 2023
610b485
Shorten `value` and `error` to `val` and `err`
zharinov Jul 15, 2023
369996e
More clarification for `wrapNullable`
zharinov Jul 15, 2023
174a2c6
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 15, 2023
2c18344
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 16, 2023
22f0afc
Remove debug line
zharinov Jul 17, 2023
2556215
Add comments and restrict error type inherence
zharinov Jul 17, 2023
8fdb428
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 17, 2023
3d9f4ef
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 17, 2023
0937d7e
Re-throw uncaught error when unwrapping result
zharinov Jul 17, 2023
d68d44a
Rename
zharinov Jul 17, 2023
eca2c46
One more comment
zharinov Jul 17, 2023
3e7036e
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 17, 2023
b8b2eb0
Doc
zharinov Jul 18, 2023
ac06b56
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 19, 2023
9ccfdff
Merge branch 'main' into feat/result-promise-transform
zharinov Jul 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 19 additions & 14 deletions lib/modules/datasource/rubygems/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Marshal } from '@qnighy/marshal';
import { logger } from '../../../logger';
import { cache } from '../../../util/cache/package/decorator';
import { HttpError } from '../../../util/http';
import { Result } from '../../../util/result';
import { getQueryString, joinUrlParts, parseUrl } from '../../../util/url';
import * as rubyVersioning from '../../versioning/ruby';
import { Datasource } from '../datasource';
Expand Down Expand Up @@ -40,23 +41,27 @@ export class RubyGemsDatasource extends Datasource {
return null;
}

try {
const { res: versionsResult } =
await this.versionsEndpointCache.getVersions(registryUrl, packageName);

if (versionsResult.success) {
const { value: versions } = versionsResult;
const result = await this.metadataCache.getRelease(
registryUrl,
packageName,
versions
);
return result;
}
const { val: rubygemsResult, err: rubygemsError } = await Result.wrap(
this.versionsEndpointCache.getVersions(registryUrl, packageName)
)
.transform((versions) =>
this.metadataCache.getRelease(registryUrl, packageName, versions)
)
.unwrap();

// istanbul ignore else: will be removed soon
if (rubygemsResult) {
return rubygemsResult;
} else if (rubygemsError instanceof Error) {
this.handleGenericErrors(rubygemsError);
}

try {
rubygemsError;
zharinov marked this conversation as resolved.
Show resolved Hide resolved
const registryHostname = parseUrl(registryUrl)?.hostname;

if (
versionsResult.error === 'unsupported-api' &&
rubygemsError === 'unsupported-api' &&
registryHostname !== 'rubygems.org'
) {
if (
Expand Down
44 changes: 22 additions & 22 deletions lib/modules/datasource/rubygems/versions-endpoint-cache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
const baz = await rubygems.getVersions(registryUrl, 'baz');
const qux = await rubygems.getVersions(registryUrl, 'qux');

expect(foo.value).toEqual(['1.1.1']);
expect(bar.value).toEqual(['2.2.2']);
expect(baz.value).toEqual(['3.3.3']);
expect(qux.error).toBe('package-not-found');
expect(foo.unwrap().val).toEqual(['1.1.1']);
expect(bar.unwrap().val).toEqual(['2.2.2']);
expect(baz.unwrap().val).toEqual(['3.3.3']);
expect(qux.unwrap().err).toBe('package-not-found');

expect(memCache.get('https://rubygems.org')?.value).toMatchObject({
expect(memCache.get('https://rubygems.org')?.unwrap().val).toMatchObject({
contentTail: '33333333333333333333333333333333\n',
});
});
Expand All @@ -49,19 +49,19 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
rubygems.getVersions(registryUrl, 'baz'),
]);

expect(foo.value).toEqual(['1.1.1']);
expect(bar.value).toEqual(['2.2.2']);
expect(baz.value).toEqual(['3.3.3']);
expect(foo.unwrap().val).toEqual(['1.1.1']);
expect(bar.unwrap().val).toEqual(['2.2.2']);
expect(baz.unwrap().val).toEqual(['3.3.3']);
});

it('handles 404', async () => {
httpMock.scope(registryUrl).get('/versions').reply(404);

const res1 = await rubygems.getVersions(registryUrl, 'foo');
expect(res1.error).toBe('unsupported-api');
expect(res1.unwrap().err).toBe('unsupported-api');

const res2 = await rubygems.getVersions(registryUrl, 'foo');
expect(res2.error).toBe('unsupported-api');
expect(res2.unwrap().err).toBe('unsupported-api');

expect(memCache.size).toBe(1);
});
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
httpMock.scope(registryUrl).get('/versions').reply(200, fullBody);

const res1 = await rubygems.getVersions(registryUrl, 'foo');
expect(res1.value).toEqual(['1.1.1']);
expect(res1.unwrap().val).toEqual(['1.1.1']);

jest.advanceTimersByTime(15 * 60 * 1000);
httpMock
Expand All @@ -107,9 +107,9 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
);

const res2 = await rubygems.getVersions(registryUrl, 'foo');
expect(res2.value).toEqual(['1.2.3']);
expect(res2.unwrap().val).toEqual(['1.2.3']);

expect(memCache.get('https://rubygems.org')?.value).toMatchObject({
expect(memCache.get('https://rubygems.org')?.unwrap().val).toMatchObject({
contentTail: '44444444444444444444444444444444\n',
});
});
Expand All @@ -118,7 +118,7 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
httpMock.scope(registryUrl).get('/versions').reply(200, fullBody);

const res1 = await rubygems.getVersions(registryUrl, 'foo');
expect(res1.value).toEqual(['1.1.1']);
expect(res1.unwrap().val).toEqual(['1.1.1']);

jest.advanceTimersByTime(15 * 60 * 1000);
httpMock
Expand All @@ -144,9 +144,9 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
);

const res2 = await rubygems.getVersions(registryUrl, 'foo');
expect(res2.value).toEqual(['1.2.3']);
expect(res2.unwrap().val).toEqual(['1.2.3']);

expect(memCache.get('https://rubygems.org')?.value).toMatchObject({
expect(memCache.get('https://rubygems.org')?.unwrap().val).toMatchObject({
contentTail: '01010101010101010101010101010101\n',
});
});
Expand All @@ -155,7 +155,7 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
httpMock.scope(registryUrl).get('/versions').reply(200, fullBody);

const res1 = await rubygems.getVersions(registryUrl, 'foo');
expect(res1.value).toEqual(['1.1.1']);
expect(res1.unwrap().val).toEqual(['1.1.1']);

jest.advanceTimersByTime(15 * 60 * 1000);
httpMock
Expand All @@ -167,9 +167,9 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
);

const res2 = await rubygems.getVersions(registryUrl, 'foo');
expect(res2.value).toEqual(['1.2.3']);
expect(res2.unwrap().val).toEqual(['1.2.3']);

expect(memCache.get('https://rubygems.org')?.value).toMatchObject({
expect(memCache.get('https://rubygems.org')?.unwrap().val).toMatchObject({
contentTail: '44444444444444444444444444444444\n',
});
});
Expand All @@ -187,10 +187,10 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {
httpMock.scope(registryUrl).get('/versions').reply(404);

const res1 = await rubygems.getVersions(registryUrl, 'foo');
expect(res1.error).toBe('unsupported-api');
expect(res1.unwrap().err).toBe('unsupported-api');

const res2 = await rubygems.getVersions(registryUrl, 'foo');
expect(res2.error).toBe('unsupported-api');
expect(res2.unwrap().err).toBe('unsupported-api');
});

it('handles 416', async () => {
Expand All @@ -210,7 +210,7 @@ describe('modules/datasource/rubygems/versions-endpoint-cache', () => {

const res = await rubygems.getVersions(registryUrl, 'foo');

expect(res.value).toEqual(['9.9.9']);
expect(res.unwrap().val).toEqual(['9.9.9']);
});

it('handles unknown errors', async () => {
Expand Down
14 changes: 7 additions & 7 deletions lib/modules/datasource/rubygems/versions-endpoint-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,20 @@ export class VersionsEndpointCache {
*/
private async getCache(registryUrl: string): Promise<VersionsEndpointResult> {
const oldResult = memCache.get(registryUrl);

if (!oldResult) {
const newResult = await this.fullSync(registryUrl);
cacheResult(registryUrl, newResult);
return newResult;
}

if (!oldResult.res.success) {
const { val: data } = oldResult.unwrap();
if (!data) {
return oldResult;
}

if (isStale(oldResult.res.value)) {
if (isStale(data)) {
memCache.delete(registryUrl); // If no error is thrown, we'll re-set the cache
const newResult = await this.deltaSync(oldResult.res.value, registryUrl);
const newResult = await this.deltaSync(data, registryUrl);
cacheResult(registryUrl, newResult);
return newResult;
}
Expand All @@ -182,17 +182,17 @@ export class VersionsEndpointCache {
} finally {
this.cacheRequests.delete(registryUrl);
}
const { res } = cachedResult;

if (!res.success) {
const { val: cachedData } = cachedResult.unwrap();
if (!cachedData) {
logger.debug(
{ packageName, registryUrl },
'Rubygems: endpoint not supported'
);
return Result.err('unsupported-api');
}

const versions = res.value.packageVersions.get(packageName);
const versions = cachedData.packageVersions.get(packageName);
if (!versions?.length) {
logger.debug(
{ packageName, registryUrl },
Expand Down