Skip to content

Allow use of multiple extension galleries #2620

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+45 −30
Closed

Allow use of multiple extension galleries #2620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 42 additions & 27 deletions lib/vscode/src/vs/platform/extensionManagement/common/extensionGalleryService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ export class ExtensionGalleryService implements IExtensionGalleryService {

declare readonly _serviceBrand: undefined;

private extensionsGalleryUrl: string | undefined;
private extensionsGalleryUrl: string[] | undefined;
private extensionsControlUrl: string | undefined;

private readonly commonHeadersPromise: Promise<{ [key: string]: string; }>;
Expand All @@ -363,8 +363,13 @@ export class ExtensionGalleryService implements IExtensionGalleryService {
this.commonHeadersPromise = resolveMarketplaceHeaders(productService.version, this.environmentService, this.fileService, storageService);
}

private api(path = ''): string {
return `${this.extensionsGalleryUrl}${path}`;
private api(path = ''): string[] {
if (!!this.extensionsGalleryUrl) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the double ! to convert it to a boolean? Does it have a performance advantage over the Boolean function? Only asking/suggesting if it might improve readability - obviously your call :D

Suggested change
if (!!this.extensionsGalleryUrl) {
if (Boolean(this.extensionsGalleryUrl)) {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I should just use this.isEnabled() instead - does the same check in practice but is probably ideal since the code already exists.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking!

return this.extensionsGalleryUrl?.map(
(galleryUrl) => `${galleryUrl}${path}`
);
}
return [];
}

isEnabled(): boolean {
Expand Down Expand Up @@ -543,43 +548,53 @@ export class ExtensionGalleryService implements IExtensionGalleryService {
'Content-Length': String(data.length)
};

const context = await this.requestService.request({
type: 'POST',
url: this.api('/extensionquery'),
data,
headers
}, token);
const galleryExtensions: IRawGalleryExtension[] = [];
const seenExtensions: Set<String> = new Set();

if (context.res.statusCode && context.res.statusCode >= 400 && context.res.statusCode < 500) {
return { galleryExtensions: [], total: 0 };
}
for (const url of this.api('/extensionquery')) {
const context = await this.requestService.request({
type: 'POST',
url: url,
data,
headers
}, token);

const result = await asJson<IRawGalleryQueryResult>(context);
if (result) {
const r = result.results[0];
const galleryExtensions = r.extensions;
const resultCount = r.resultMetadata && r.resultMetadata.filter(m => m.metadataType === 'ResultCount')[0];
const total = resultCount && resultCount.metadataItems.filter(i => i.name === 'TotalCount')[0].count || 0;
if (context.res.statusCode && context.res.statusCode >= 400 && context.res.statusCode < 500) {
return { galleryExtensions: [], total: 0 };
}

return { galleryExtensions, total };
const result = await asJson<IRawGalleryQueryResult>(context);
if (result) {
const r = result.results[0];
for (const extension of r.extensions) {
if (!seenExtensions.has(extension.extensionId)) {
galleryExtensions.push(extension);
seenExtensions.add(extension.extensionId);
}
}
}
}
return { galleryExtensions: [], total: 0 };
return { galleryExtensions, total: galleryExtensions.length };
}

async reportStatistic(publisher: string, name: string, version: string, type: StatisticType): Promise<void> {
// TODO: investigate further - currently we just send stats everywhere
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know folks have GitLens and tools to know who left a TODO comment but at least this puts your name right there so we don't forget.

Suggested change
// TODO: investigate further - currently we just send stats everywhere
// TODO(oxy): investigate further - currently we just send stats everywhere

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 will fix!

// this is only used in one place - uninstall tracking - but
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfinished comment? Was there more to this sentence?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, there was!
was supposed to say but unsure if MS will start using this elsewhere

if (!this.isEnabled()) {
return undefined;
}

const commonHeaders = await this.commonHeadersPromise;
const headers = { ...commonHeaders, Accept: '*/*;api-version=4.0-preview.1' };
try {
await this.requestService.request({
type: 'POST',
url: this.api(`/publishers/${publisher}/extensions/${name}/${version}/stats?statType=${type}`),
headers
}, CancellationToken.None);
} catch (error) { /* Ignore */ }
for (const url of this.api(`/publishers/${publisher}/extensions/${name}/${version}/stats?statType=${type}`)) {
try {
await this.requestService.request({
type: 'POST',
url: url,
headers
}, CancellationToken.None);
} catch (error) { /* Ignore */ }
}
}

async download(extension: IGalleryExtension, location: URI, operation: InstallOperation): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion lib/vscode/src/vs/platform/product/common/productService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export interface IProductConfiguration {
readonly experimentsUrl?: string;

readonly extensionsGallery?: {
readonly serviceUrl: string;
readonly serviceUrl: string[];
readonly itemUrl: string;
readonly controlUrl: string;
readonly recommendationsUrl: string;
Expand Down
2 changes: 1 addition & 1 deletion lib/vscode/src/vs/server/ipc.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export interface WorkbenchOptions {
readonly productConfiguration: {
codeServerVersion?: string;
readonly extensionsGallery?: {
readonly serviceUrl: string;
readonly serviceUrl: string[];
readonly itemUrl: string;
readonly controlUrl: string;
readonly recommendationsUrl: string;
Expand Down
2 changes: 1 addition & 1 deletion lib/vscode/src/vs/server/node/marketplace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ const extractTar = async (tarPath: string, targetPath: string, options: IExtract
*/
export const enableCustomMarketplace = (): void => {
(<any>product).extensionsGallery = { // Use `any` to override readonly.
serviceUrl: process.env.SERVICE_URL || 'https://extensions.coder.com/api',
serviceUrl: process.env.SERVICE_URL ? [process.env.SERVICE_URL] : ['https://open-vsx.org/vscode/gallery', 'https://extensions.coder.com/api'],
itemUrl: process.env.ITEM_URL || '',
controlUrl: '',
recommendationsUrl: '',
Expand Down