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(rubygems): Unify fetching via v1 API #23517

Merged
merged 21 commits into from
Jul 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
18d50e6
refactor(rubygems): Extract v1 API handling
zharinov Jul 20, 2023
01d07fd
feat: Add `assignKeys` utility function
zharinov Jul 20, 2023
7c66dfa
Fix
zharinov Jul 20, 2023
af7551f
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 20, 2023
ebf0b49
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 21, 2023
3a30944
fix: Better types for `assignKeys` utility (#23496)
zharinov Jul 21, 2023
18d0bfb
refactor: Make `AsyncResult` a `PromiseLike`
zharinov Jul 21, 2023
c933452
Unified signature for transform
zharinov Jul 21, 2023
08e4fa2
feat: Support `.catch` method for `Result`
zharinov Jul 21, 2023
665a4aa
Refactor
zharinov Jul 21, 2023
e135d0f
Fix
zharinov Jul 21, 2023
dab947c
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 21, 2023
2289ec2
Merge branch 'main' of github.com:renovatebot/renovate into refactor/…
zharinov Jul 22, 2023
6a1469b
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 22, 2023
bcb7667
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 22, 2023
de79cb9
Refactor
zharinov Jul 22, 2023
2f81add
Merge branch 'main' into refactor/rubygems-extract-v1-fetch
zharinov Jul 23, 2023
21db221
refactor(rubygems): Unify fetching via v1 API
zharinov Jul 23, 2023
ef6195a
Merge branch 'main' into refactor/unify-v1-fetching
zharinov Jul 23, 2023
ce1238b
Merge branch 'main' of github.com:renovatebot/renovate into refactor/…
zharinov Jul 23, 2023
6752773
Merge branch 'refactor/unify-v1-fetching' of github.com:zharinov/reno…
zharinov Jul 23, 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
28 changes: 24 additions & 4 deletions lib/modules/datasource/rubygems/common.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { assignKeys } from '../../../util/assign-keys';
import type { Http, SafeJsonError } from '../../../util/http';
import type { AsyncResult } from '../../../util/result';
import { type Http, HttpError, type SafeJsonError } from '../../../util/http';
import { type AsyncResult, Result } from '../../../util/result';
import { joinUrlParts as join } from '../../../util/url';
import type { Release, ReleaseResult } from '../types';
import { GemMetadata, GemVersions } from './schema';
Expand All @@ -9,7 +9,10 @@ export function getV1Releases(
http: Http,
registryUrl: string,
packageName: string
): AsyncResult<ReleaseResult, SafeJsonError> {
): AsyncResult<
ReleaseResult,
SafeJsonError | 'empty-releases' | 'unsupported-api'
> {
const fileName = `${packageName}.json`;
const versionsUrl = join(registryUrl, '/api/v1/versions', fileName);
const metadataUrl = join(registryUrl, '/api/v1/gems', fileName);
Expand All @@ -26,5 +29,22 @@ export function getV1Releases(
)
.unwrap({ releases });

return http.getJsonSafe(versionsUrl, GemVersions).transform(addMetadata);
return http
.getJsonSafe(versionsUrl, GemVersions)
.catch((err) => {
if (err instanceof HttpError) {
const status = err.response?.statusCode;
if (status === 404 || status === 400) {
return Result.err('unsupported-api');
}
}

return Result.err(err);
})
.transform((releases) => {
return releases.length > 0
? Result.ok(releases)
: Result.err('empty-releases');
})
.transform(addMetadata);
}
56 changes: 24 additions & 32 deletions lib/modules/datasource/rubygems/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,19 @@ describe('modules/datasource/rubygems/index', () => {
httpMock
.scope('https://firstparty.com')
.get('/basepath/versions')
.reply(404);
httpMock
.scope('https://firstparty.com')
.get('/basepath/api/v1/gems/rails.json')
.reply(200, { name: 'rails' })
.reply(404)
.get('/basepath/api/v1/versions/rails.json')
.reply(200, []);
httpMock.scope('https://thirdparty.com').get('/versions').reply(404);
.reply(200, [])
.get('/basepath/api/v1/dependencies?gems=rails')
.reply(200, emptyMarshalArray);
httpMock
.scope('https://thirdparty.com')
.get('/api/v1/gems/rails.json')
.reply(200, { name: 'rails' })
.get('/versions')
.reply(404)
.get('/api/v1/versions/rails.json')
.reply(200, []);
.reply(200, [])
.get('/api/v1/dependencies?gems=rails')
.reply(200, emptyMarshalArray);
expect(
await getPkgReleases({
versioning: rubyVersioning.id,
Expand Down Expand Up @@ -151,16 +150,18 @@ describe('modules/datasource/rubygems/index', () => {
.scope('https://thirdparty.com/')
.get('/versions')
.reply(404)
.get('/api/v1/gems/rails.json')
.reply(401);
.get('/api/v1/versions/rails.json')
.reply(400)
.get('/api/v1/dependencies?gems=rails')
.reply(200, emptyMarshalArray);
httpMock
.scope('https://firstparty.com/')
.get('/basepath/versions')
.reply(404)
.get('/basepath/api/v1/gems/rails.json')
.reply(200, railsInfo)
.get('/basepath/api/v1/versions/rails.json')
.reply(200, railsVersions);
.reply(200, railsVersions)
.get('/basepath/api/v1/gems/rails.json')
.reply(200, railsInfo);

const res = await getPkgReleases({
versioning: rubyVersioning.id,
Expand All @@ -175,35 +176,29 @@ describe('modules/datasource/rubygems/index', () => {
expect(res).toMatchSnapshot();
});

it('falls back to info when version request fails', async () => {
it('falls back to dependencies when other API requests fail', async () => {
httpMock
.scope('https://thirdparty.com/')
.get('/versions')
.reply(404)
.get('/api/v1/gems/rails.json')
.reply(200, railsInfo)
.get('/api/v1/versions/rails.json')
.reply(400, {});
.reply(400, {})
.get('/api/v1/dependencies?gems=rails')
.reply(200, railsDependencies);
const res = await getPkgReleases({
versioning: rubyVersioning.id,
datasource: RubyGemsDatasource.id,
packageName: 'rails',
registryUrls: [
'https://thirdparty.com',
'https://firstparty.com/basepath/',
],
registryUrls: ['https://thirdparty.com'],
});
expect(res?.releases).toHaveLength(1);
expect(res?.releases[0].version).toBe(railsInfo.version);
expect(res?.releases).toHaveLength(339);
});

it('errors when version request fails with anything other than 400 or 404', async () => {
httpMock
.scope('https://thirdparty.com/')
.get('/versions')
.reply(404)
.get('/api/v1/gems/rails.json')
.reply(200, railsInfo)
.get('/api/v1/versions/rails.json')
.reply(500, {});
await expect(
Expand All @@ -224,7 +219,7 @@ describe('modules/datasource/rubygems/index', () => {
.scope('https://thirdparty.com/')
.get('/versions')
.reply(404)
.get('/api/v1/gems/rails.json')
.get('/api/v1/versions/rails.json')
.reply(404, railsInfo)
.get('/api/v1/dependencies?gems=rails')
.reply(200, railsDependencies);
Expand All @@ -233,10 +228,7 @@ describe('modules/datasource/rubygems/index', () => {
versioning: rubyVersioning.id,
datasource: RubyGemsDatasource.id,
packageName: 'rails',
registryUrls: [
'https://thirdparty.com',
'https://firstparty.com/basepath/',
],
registryUrls: ['https://thirdparty.com'],
});
expect(res?.releases).toHaveLength(339);
});
Expand Down
120 changes: 11 additions & 109 deletions lib/modules/datasource/rubygems/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
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';
import type { GetReleasesConfig, Release, ReleaseResult } from '../types';
import type { GetReleasesConfig, ReleaseResult } from '../types';
import { getV1Releases } from './common';
import { RubygemsHttp } from './http';
import { MetadataCache } from './metadata-cache';
import { GemMetadata, GemVersions, MarshalledVersionInfo } from './schema';
import { MarshalledVersionInfo } from './schema';
import { VersionsEndpointCache } from './versions-endpoint-cache';

export class RubyGemsDatasource extends Datasource {
Expand Down Expand Up @@ -70,19 +69,18 @@ export class RubyGemsDatasource extends Datasource {
return await this.getReleasesViaFallbackAPI(registryUrl, packageName);
}

const gemMetadata = await this.fetchGemMetadata(
const { val: apiV1Result, err: apiV1Error } = await getV1Releases(
this.http,
registryUrl,
packageName
);
if (!gemMetadata) {
return await this.getReleasesViaFallbackAPI(registryUrl, packageName);
).unwrap();
if (apiV1Result) {
return apiV1Result;
} else if (apiV1Error instanceof HttpError) {
throw apiV1Error;
}

return await this.getReleasesViaAPI(
registryUrl,
packageName,
gemMetadata
);
return await this.getReleasesViaFallbackAPI(registryUrl, packageName);
}

return null;
Expand All @@ -91,102 +89,6 @@ export class RubyGemsDatasource extends Datasource {
}
}

@cache({
namespace: `datasource-${RubyGemsDatasource.id}`,
key: ({ registryUrl, packageName }: GetReleasesConfig) =>
// TODO: types (#7154)
/* eslint-disable @typescript-eslint/restrict-template-expressions */
`metadata:${registryUrl}/${packageName}`,
})
async fetchGemMetadata(
registryUrl: string,
packageName: string
): Promise<GemMetadata | null> {
try {
const { body } = await this.http.getJson(
joinUrlParts(registryUrl, '/api/v1/gems', `${packageName}.json`),
GemMetadata
);
return body;
} catch (err) {
// fallback to deps api on 404
if (err instanceof HttpError && err.response?.statusCode === 404) {
return null;
}
throw err;
}
}

@cache({
namespace: `datasource-${RubyGemsDatasource.id}`,
key: ({ registryUrl, packageName }: GetReleasesConfig) =>
// TODO: types (#7154)
/* eslint-disable @typescript-eslint/restrict-template-expressions */
`versions:${registryUrl}/${packageName}`,
})
async fetchGemVersions(
registryUrl: string,
packageName: string
): Promise<GemVersions | null> {
try {
const { body } = await this.http.getJson(
joinUrlParts(registryUrl, '/api/v1/versions', `${packageName}.json`),
GemVersions
);
return body;
} catch (err) {
if (err.statusCode === 400 || err.statusCode === 404) {
logger.debug(
{ registry: registryUrl },
'versions endpoint returns error - falling back to info endpoint'
);
return null;
} else {
throw err;
}
}
}

async getReleasesViaAPI(
registryUrl: string,
packageName: string,
gemMetadata: GemMetadata
): Promise<ReleaseResult | null> {
const gemVersions = await this.fetchGemVersions(registryUrl, packageName);

let releases: Release[] | null = null;
if (gemVersions?.length) {
releases = gemVersions;
} else if (gemMetadata.latestVersion) {
releases = [{ version: gemMetadata.latestVersion }];
} else {
return null;
}

const result: ReleaseResult = { releases };

if (gemMetadata.changelogUrl) {
result.changelogUrl = gemMetadata.changelogUrl;
}

if (gemMetadata.homepage) {
result.homepage = gemMetadata.homepage;
}

if (gemMetadata.sourceUrl) {
result.sourceUrl = gemMetadata.sourceUrl;
}

return result;
}

@cache({
viceice marked this conversation as resolved.
Show resolved Hide resolved
namespace: `datasource-${RubyGemsDatasource.id}`,
key: ({ registryUrl, packageName }: GetReleasesConfig) =>
// TODO: types (#7154)
/* eslint-disable @typescript-eslint/restrict-template-expressions */
`dependencies:${registryUrl}/${packageName}`,
})
async getReleasesViaFallbackAPI(
registryUrl: string,
packageName: string
Expand Down