vsx-registry: Skip extension resolution if already installed (#10624)

* Existing extensions are skipped on extension download * All extensions are considered for dependencies during startup
eclipse-theia · Jan 21, 2022 · d6e4971 · d6e4971
1 parent 4bf7a93
commit d6e4971
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - [core] `ContextKeyService` is now an interface. Extenders should extend `ContextKeyServiceDummyImpl` [#10546](https://github.com/eclipse-theia/theia/pull/10546)
 - [plugin] Removed deprecated fields `id` and `label` from `theia.Command` [#10512](https://github.com/eclipse-theia/theia/pull/10512)
 - [plugin-ext] `ViewContextKeyService#with` method removed. Use `ContextKeyService#with` instead. `PluginViewWidget` and `PluginTreeWidget` inject the `ContextKeyService` rather than `ViewContextKeyService`. [#10546](https://github.com/eclipse-theia/theia/pull/10546)
+- [plugin-ext] `PluginDeployerImpl` now uses the `UnresolvedPluginEntry: { id: string, type: PluginType }` interface as parameter types for resolving plugins. Affected methods: `deploy`, `deployMultipleEntries` and `resolvePlugins`.
 - [navigator] added `Open Containing Folder` command [#10523](https://github.com/eclipse-theia/theia/pull/10523)
 - [core] Removed deprecated API: `unfocusSearchFieldContainer`, `doUnfocusSearchFieldContainer()` [#10625](https://github.com/eclipse-theia/theia/pull/10625)
 

diff --git a/packages/plugin-ext/src/common/plugin-protocol.ts b/packages/plugin-ext/src/common/plugin-protocol.ts
@@ -406,6 +406,11 @@ export enum PluginType {
     User
 };
 
+export interface UnresolvedPluginEntry {
+    id: string;
+    type?: PluginType;
+}
+
 export interface PluginDeployerEntry {
 
     /**
@@ -822,9 +827,10 @@ export interface PluginDeployerHandler {
     deployFrontendPlugins(frontendPlugins: PluginDeployerEntry[]): Promise<void>;
     deployBackendPlugins(backendPlugins: PluginDeployerEntry[]): Promise<void>;
 
+    getDeployedPlugin(pluginId: string): DeployedPlugin | undefined;
     undeployPlugin(pluginId: string): Promise<boolean>;
 
-    getPluginDependencies(pluginToBeInstalled: PluginDeployerEntry): Promise<PluginDependencies | undefined>
+    getPluginDependencies(pluginToBeInstalled: PluginDeployerEntry): Promise<PluginDependencies | undefined>;
 }
 
 export interface GetDeployedPluginsParams {

diff --git a/packages/plugin-ext/src/main/node/plugin-deployer-impl.ts b/packages/plugin-ext/src/main/node/plugin-deployer-impl.ts
@@ -21,7 +21,7 @@ import {
     PluginDeployerResolver, PluginDeployerFileHandler, PluginDeployerDirectoryHandler,
     PluginDeployerEntry, PluginDeployer, PluginDeployerParticipant, PluginDeployerStartContext,
     PluginDeployerResolverInit, PluginDeployerFileHandlerContext,
-    PluginDeployerDirectoryHandlerContext, PluginDeployerEntryType, PluginDeployerHandler, PluginType
+    PluginDeployerDirectoryHandlerContext, PluginDeployerEntryType, PluginDeployerHandler, PluginType, UnresolvedPluginEntry
 } from '../../common/plugin-protocol';
 import { PluginDeployerEntryImpl } from './plugin-deployer-entry-impl';
 import {
@@ -118,11 +118,16 @@ export class PluginDeployerImpl implements PluginDeployer {
         }
 
         const startDeployTime = performance.now();
-        const [userPlugins, systemPlugins] = await Promise.all([
-            this.resolvePlugins(context.userEntries, PluginType.User),
-            this.resolvePlugins(context.systemEntries, PluginType.System)
-        ]);
-        await this.deployPlugins([...userPlugins, ...systemPlugins]);
+        const unresolvedUserEntries = context.userEntries.map(id => ({
+            id,
+            type: PluginType.User
+        }));
+        const unresolvedSystemEntries = context.systemEntries.map(id => ({
+            id,
+            type: PluginType.System
+        }));
+        const plugins = await this.resolvePlugins([...unresolvedUserEntries, ...unresolvedSystemEntries]);
+        await this.deployPlugins(plugins);
         this.logMeasurement('Deploy plugins list', startDeployTime);
     }
 
@@ -132,69 +137,75 @@ export class PluginDeployerImpl implements PluginDeployer {
         }
     }
 
-    async deploy(pluginEntry: string, type: PluginType = PluginType.System): Promise<void> {
+    async deploy(plugin: UnresolvedPluginEntry): Promise<void> {
         const startDeployTime = performance.now();
-        await this.deployMultipleEntries([pluginEntry], type);
+        await this.deployMultipleEntries([plugin]);
         this.logMeasurement('Deploy plugin entry', startDeployTime);
     }
 
-    protected async deployMultipleEntries(pluginEntries: ReadonlyArray<string>, type: PluginType = PluginType.System): Promise<void> {
-        const pluginsToDeploy = await this.resolvePlugins(pluginEntries, type);
+    protected async deployMultipleEntries(plugins: UnresolvedPluginEntry[]): Promise<void> {
+        const pluginsToDeploy = await this.resolvePlugins(plugins);
         await this.deployPlugins(pluginsToDeploy);
     }
 
     /**
      * Resolves plugins for the given type.
      *
-     * One can call it multiple times for different types before triggering a single deploy, i.e.
+     * Only call it a single time before triggering a single deploy to prevent re-resolving of extension dependencies, i.e.
      * ```ts
      * const deployer: PluginDeployer;
-     * deployer.deployPlugins([
-     *     ...await deployer.resolvePlugins(userEntries, PluginType.User),
-     *     ...await deployer.resolvePlugins(systemEntries, PluginType.System)
-     * ]);
+     * deployer.deployPlugins(await deployer.resolvePlugins(allPluginEntries));
      * ```
      */
-    async resolvePlugins(pluginEntries: ReadonlyArray<string>, type: PluginType): Promise<PluginDeployerEntry[]> {
+    async resolvePlugins(plugins: UnresolvedPluginEntry[]): Promise<PluginDeployerEntry[]> {
         const visited = new Set<string>();
         const pluginsToDeploy = new Map<string, PluginDeployerEntry>();
 
-        let queue = [...pluginEntries];
+        let queue: UnresolvedPluginEntry[] = [...plugins];
         while (queue.length) {
-            const dependenciesChunk: Array<Map<string, string>> = [];
-            const workload: string[] = [];
+            const dependenciesChunk: Array<{
+                dependencies: Map<string, string>
+                type: PluginType
+            }> = [];
+            const workload: UnresolvedPluginEntry[] = [];
             while (queue.length) {
                 const current = queue.shift()!;
-                if (visited.has(current)) {
+                if (visited.has(current.id)) {
                     continue;
                 } else {
                     workload.push(current);
                 }
-                visited.add(current);
+                visited.add(current.id);
             }
             queue = [];
-            await Promise.all(workload.map(async current => {
+            await Promise.all(workload.map(async ({ id, type }) => {
+                if (type === undefined) {
+                    type = PluginType.System;
+                }
                 try {
-                    const pluginDeployerEntries = await this.resolvePlugin(current, type);
+                    const pluginDeployerEntries = await this.resolvePlugin(id, type);
                     await this.applyFileHandlers(pluginDeployerEntries);
                     await this.applyDirectoryFileHandlers(pluginDeployerEntries);
                     for (const deployerEntry of pluginDeployerEntries) {
                         const dependencies = await this.pluginDeployerHandler.getPluginDependencies(deployerEntry);
                         if (dependencies && !pluginsToDeploy.has(dependencies.metadata.model.id)) {
                             pluginsToDeploy.set(dependencies.metadata.model.id, deployerEntry);
                             if (dependencies.mapping) {
-                                dependenciesChunk.push(dependencies.mapping);
+                                dependenciesChunk.push({ dependencies: dependencies.mapping, type });
                             }
                         }
                     }
                 } catch (e) {
-                    console.error(`Failed to resolve plugins from '${current}'`, e);
+                    console.error(`Failed to resolve plugins from '${id}'`, e);
                 }
             }));
-            for (const dependencies of dependenciesChunk) {
+            for (const { dependencies, type } of dependenciesChunk) {
                 for (const [dependency, deployableDependency] of dependencies) {
                     if (!pluginsToDeploy.has(dependency)) {
-                        queue.push(deployableDependency);
+                        queue.push({
+                            id: deployableDependency,
+                            type
+                        });
                     }
                 }
             }

diff --git a/packages/plugin-ext/src/main/node/plugin-server-handler.ts b/packages/plugin-ext/src/main/node/plugin-server-handler.ts
@@ -18,7 +18,7 @@ import { injectable, inject } from '@theia/core/shared/inversify';
 import { CancellationToken } from '@theia/core/lib/common/cancellation';
 import { PluginDeployerImpl } from './plugin-deployer-impl';
 import { PluginsKeyValueStorage } from './plugins-key-value-storage';
-import { PluginServer, PluginDeployer, PluginStorageKind, PluginType } from '../../common/plugin-protocol';
+import { PluginServer, PluginDeployer, PluginStorageKind, PluginType, UnresolvedPluginEntry } from '../../common/plugin-protocol';
 import { KeysToAnyValues, KeysToKeysToAnyValue } from '../../common/types';
 
 @injectable()
@@ -32,10 +32,14 @@ export class PluginServerHandler implements PluginServer {
 
     deploy(pluginEntry: string, arg2?: PluginType | CancellationToken): Promise<void> {
         const type = typeof arg2 === 'number' ? arg2 as PluginType : undefined;
-        return this.doDeploy(pluginEntry, type);
+        return this.doDeploy({
+            id: pluginEntry,
+            type: type ?? PluginType.User
+        });
     }
-    protected doDeploy(pluginEntry: string, type: PluginType = PluginType.User): Promise<void> {
-        return this.pluginDeployer.deploy(pluginEntry, type);
+
+    protected doDeploy(pluginEntry: UnresolvedPluginEntry): Promise<void> {
+        return this.pluginDeployer.deploy(pluginEntry);
     }
 
     undeploy(pluginId: string): Promise<void> {

diff --git a/packages/vsx-registry/src/node/vsx-extension-resolver.ts b/packages/vsx-registry/src/node/vsx-extension-resolver.ts
@@ -16,21 +16,26 @@
 
 import * as os from 'os';
 import * as path from 'path';
+import * as semver from 'semver';
 import * as fs from '@theia/core/shared/fs-extra';
 import { v4 as uuidv4 } from 'uuid';
 import * as requestretry from 'requestretry';
 import { injectable, inject } from '@theia/core/shared/inversify';
 import URI from '@theia/core/lib/common/uri';
-import { PluginDeployerResolver, PluginDeployerResolverContext } from '@theia/plugin-ext/lib/common/plugin-protocol';
+import { PluginDeployerHandler, PluginDeployerResolver, PluginDeployerResolverContext } from '@theia/plugin-ext/lib/common/plugin-protocol';
 import { VSXExtensionUri } from '../common/vsx-extension-uri';
 import { OVSXClientProvider } from '../common/ovsx-client-provider';
+import { VSXExtensionRaw } from '@theia/ovsx-client';
 
 @injectable()
 export class VSXExtensionResolver implements PluginDeployerResolver {
 
     @inject(OVSXClientProvider)
     protected clientProvider: OVSXClientProvider;
 
+    @inject(PluginDeployerHandler)
+    protected pluginDeployerHandler: PluginDeployerHandler;
+
     protected readonly downloadPath: string;
 
     constructor() {
@@ -61,6 +66,12 @@ export class VSXExtensionResolver implements PluginDeployerResolver {
         const downloadUrl = extension.files.download;
         console.log(`[${id}]: resolved to '${resolvedId}'`);
 
+        const existingVersion = this.hasSameOrNewerVersion(id, extension);
+        if (existingVersion) {
+            console.log(`[${id}]: is already installed with the same or newer version '${existingVersion}'`);
+            return;
+        }
+
         const extensionPath = path.resolve(this.downloadPath, path.basename(downloadUrl));
         console.log(`[${resolvedId}]: trying to download from "${downloadUrl}"...`);
         if (!await this.download(downloadUrl, extensionPath)) {
@@ -71,6 +82,18 @@ export class VSXExtensionResolver implements PluginDeployerResolver {
         context.addPlugin(resolvedId, extensionPath);
     }
 
+    protected hasSameOrNewerVersion(id: string, extension: VSXExtensionRaw): string | undefined {
+        const existingPlugin = this.pluginDeployerHandler.getDeployedPlugin(id);
+        if (existingPlugin) {
+            const existingVersion = semver.clean(existingPlugin.metadata.model.version);
+            const desiredVersion = semver.clean(extension.version);
+            if (desiredVersion && existingVersion && semver.gte(existingVersion, desiredVersion)) {
+                return existingVersion;
+            }
+        }
+        return undefined;
+    }
+
     protected async download(downloadUrl: string, downloadPath: string): Promise<boolean> {
         return new Promise<boolean>((resolve, reject) => {
             requestretry(downloadUrl, {